-
Notifications
You must be signed in to change notification settings - Fork 152
Move payload codec, encode, and decode calls to DataConverter helper methods #1305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this change of moving the payload-to/from-payload encoding/decoding to the data converter now that we're going to do more than just codec application (will also do limit checking and external payload provider stuff).
Nothing blocking from my POV, but not approving so Python SDK owners can look/approve (cc @tconley1428 and @THardy98)
| return await self.payload_codec.encode(payloads) | ||
|
|
||
| # Temporary shortcircuit detection while the _encode_* methods may no-op if | ||
| # a payload codec is not configured. Remove once those paths have more to them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or potentially keep but extend logic to include those other things. If there is still a scenario where it does nothing.
What was changed
Move payload codec, encode, and decode calls to DataConverter helper methods.
Why?
Allow future changes to have a central place to extend the general notions of encoding and decoding in a unified place.
Checklist